fix(check-collections): report runner scan FAILED with logs on abort re-raise (SAS-13001)#2779
Conversation
…re-raise A single-contract (runner) scan runs with abort_on_first_error=True, so the first construction/verify exception re-raises out of execute_check_collections before phase 3's combined upload — the only place that otherwise ships the engine logs and marks the scan FAILED in Cloud. The exception then reaches the CLI (exit 3) and the launcher only writes it to pod logs, leaving the Cloud scan record FAILED with no logs and the failure undiagnosable. Before the abort re-raise, best-effort mark the still-PENDING runner scan FAILED with the captured logs (the shared Logs gatherer holds every construction/verify record). The re-raise contract is preserved and reporting never masks the original exception. No-op for ad-hoc runs (no scan id). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR closes a diagnosability gap for single-contract (runner) scans that run with abort_on_first_error=True: if an exception occurs during contract construction or verify(), the scan now gets marked FAILED in Soda Cloud with captured engine logs before the exception is re-raised, avoiding “FAILED with empty logs” scan records.
Changes:
- Added best-effort pre–re-raise reporting to mark the runner scan as FAILED with captured logs when
abort_on_first_errortriggers. - Ensured reporting never masks the original exception (reporting failures are swallowed and only debug-logged).
- Added a unit test asserting that an uncaught exception during
verify()results in asodaCoreMarkScanFailedrequest with a non-empty logs payload.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py | Adds a regression test ensuring uncaught verify() exceptions still mark the runner scan FAILED and include logs. |
| soda-core/src/soda_core/check_collections/session.py | Adds a helper invoked on abort-on-first-error paths to mark the scan FAILED with captured logs before re-raising. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exc=exc if isinstance(exc, Exception) else None, | ||
| ) | ||
| except Exception: | ||
| logger.debug("Could not report runner scan as failed before re-raising", exc_info=True) |
There was a problem hiding this comment.
Should this be a debug log? That doesn't show up in cloud 🤔 .
There was a problem hiding this comment.
Agreed — bumped to warning in 6030689. You're right that debug is the wrong level here: this only fires if the mark-scan-failed call itself fails, in which case the scan stays FAILED-with-no-logs in Cloud (exactly what this helper is trying to prevent), so it needs to be visible in the pod logs / Datadog. It can't go to Cloud by nature (Cloud reporting is what just failed), and warning keeps it from being drowned out while still not masking the original exception, which is re-raised regardless.
| failure is undiagnosable (SAS-13001).""" | ||
| monkeypatch.setenv("SODA_SCAN_ID", "scan-under-test") | ||
|
|
||
| def _boom(self, *args, **kwargs): |
| "Ad-hoc run has no scan id to mark failed; mark_scan_as_failed must not be used. " | ||
| f"Requests seen: {request_types}" | ||
| ) | ||
|
|
There was a problem hiding this comment.
it seems there are two failure pathways here, construction and verification, but only verification is tested, did you think about testing construction as well?
There was a problem hiding this comment.
Good call — added test_uncaught_exception_during_construction_marks_scan_failed_with_logs in 6030689 (raises inside ContractImpl.__init__ so the exception escapes phase-1 construction rather than verify). Both abort points call the same helper, so this locks in that the construction pathway reports FAILED-with-logs too. 979 unit tests green.
Addresses review on #2779: - The best-effort "could not report scan as failed" fallback logged at debug, which is easy to miss; if it fires the scan stays FAILED-with-no-logs in Cloud (the exact state this avoids), so bump it to warning for pod/Datadog visibility. Original exception is still re-raised. - The fix reports on both abort pathways (construction + verify) but only verify was tested; add a construction-phase test (exception in ContractImpl.__init__). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
What & why
SAS-13001: a post-migration contract scan failed but produced no logs in Cloud, making it undiagnosable.
Root cause is a diagnosability hole in the engine. A single-contract (runner) scan runs
execute_check_collectionswithabort_on_first_error=True, so the first exception during contract construction or verify re-raises immediately — before phase 3's combined upload, which is the only place that ships the engine's captured logs to Cloud and callsmark_scan_as_failed. The exception then reaches the CLI (exit 3) and the launcher only writes it to pod logs, so the Cloud scan record shows FAILED with an empty log payload. This turns any pre-upload crash into a silent, undiagnosable scan.Fix
Before the
abort_on_first_errorre-raise, best-effort mark the still-PENDING runner scan FAILED with the captured logs (_report_runner_scan_failed_before_reraise). The sharedLogsgatherer holds every construction/verify record (each impl is built withlogs=logs), so its records are exactly what should reach Cloud. The historical re-raise contract is preserved and reporting never masks the original exception. No-op for ad-hoc runs (no scan id).Tests
test_uncaught_exception_during_verify_marks_scan_failed_with_logs: a scan that raises during verify now marks the scan FAILED with a non-empty log payload (fails onmain, passes here).Note
This is the diagnosability fix. The underlying crash for users (most likely a migrated contract with an unsupported check type) is tracked separately — this change makes that (and any future pre-upload crash) self-diagnosing in Cloud.
🤖 Generated with Claude Code